Skip to content

Conversation

@guan404ming
Copy link

Checklist
  • npm install && npm run lint && npm test passes
  • tests are included
  • documentation is changed or added
  • commit message follows commit guidelines
Description of change

This PR replaces weak cryptographic hash functions (MD5 and SHA1) with SHA-256 across the node-gyp codebase to improve security. The changes affect hash generation for:

  • Object ID calculation in Xcode project files
  • Intermediate file naming in Makefile generation
  • GUID generation for Visual Studio projects
  • Build rule hashing in Ninja generator

Security Impact: Addresses potential security vulnerabilities by replacing deprecated hash functions that are susceptible to collision attacks.

Performance & Functionality: No impact on build speed or functionality. All existing features work exactly the same with stronger security guarantees.

Compatibility: Maintains full backward compatibility while using modern cryptographic standards.

Copy link
Member

@legendecas legendecas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind submitting changes in gyp/ to https://github.com/nodejs/gyp-next first? Thank you!

@guan404ming
Copy link
Author

Here is the pr nodejs/gyp-next#329!

@guan404ming
Copy link
Author

guan404ming commented Jan 24, 2026

I've resolved the pr conflict, thanks!

@cclauss
Copy link
Contributor

cclauss commented Jan 25, 2026

I believe this can be closed because it will come into this codebase on the next sync with gyp-next.

@cclauss cclauss requested a review from legendecas January 25, 2026 13:02
@legendecas
Copy link
Member

Synchronization done in #3273. Thank you for your work on this!

@legendecas legendecas closed this Jan 26, 2026
@guan404ming
Copy link
Author

Thanks for the review and help!

@guan404ming guan404ming deleted the sha-256 branch January 27, 2026 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants